-
Notifications
You must be signed in to change notification settings - Fork 509
fix: QoL improvements to make the SDK more stable #1014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Enhances SDK reliability by preventing resource leaks, improving error handling, and stabilizing the public API return value.
- Adds timeout-based thread shutdown and logging in LiveSpanProcessor.shutdown
- Refines TracingCore._flush_span_processors with granular exception handling and logging
- Ensures
agentops.init()consistently returns the client’sinit()result across all code paths
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_init_api_consistency.py | Adds coverage for agentops.init() return-value behavior |
| tests/unit/sdk/test_live_span_processor_lifecycle.py | Verifies enhanced shutdown logic for LiveSpanProcessor |
| tests/unit/sdk/test_core_error_handling.py | Exercises new exception-handling paths in TracingCore |
| agentops/sdk/types.py | Switches to Required[int] for mandatory config fields |
| agentops/sdk/processors.py | Implements timeout and error handling in shutdown() |
| agentops/sdk/decorators/factory.py | Extracts session‐trace logic into sync/async helpers |
| agentops/sdk/decorators/init.py | Deprecates @session in favor of @trace with warning |
| agentops/sdk/core.py | Adds detailed logging and exception branches to flush API |
| agentops/sdk/converters.py | Introduces format_trace_id helper for ID formatting |
Comments suppressed due to low confidence (2)
tests/unit/sdk/test_live_span_processor_lifecycle.py:18
- Consider patching the logger in this test to assert that no warning or error is logged during a normal shutdown, ensuring false-positive logs are caught.
def test_shutdown_normal_thread_termination(self):
agentops/sdk/decorators/init.py:21
- There’s no existing test verifying that the deprecated
@sessiondecorator emits aDeprecationWarning. Adding a unit test for that warning would ensure the deprecation path stays valid.
# For backward compatibility: @session decorator calls @trace decorator with deprecation warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Enhancements focus on improving the SDK’s stability by preventing resource leaks, strengthening error handling, and ensuring consistent API behavior. Key changes include:
- Enhancements to LiveSpanProcessor.shutdown() with timeout-based thread join and improved logging.
- Comprehensive error handling improvements in TracingCore._flush_span_processors().
- API consistency fixes for agentops.init() with associated test updates.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_init_api_consistency.py | Added extensive tests for various init() parameter and return patterns. |
| tests/unit/sdk/test_live_span_processor_lifecycle.py | Added tests for graceful shutdown and timeout handling for span processor. |
| tests/unit/sdk/test_core_error_handling.py | Expanded tests for error handling in _flush_span_processors. |
| agentops/sdk/types.py | Updated TypedDict definitions using new Required type hints. |
| agentops/sdk/processors.py | Refactored shutdown logic with timeout, exception handling and logging. |
| agentops/sdk/decorators/factory.py | Replaced inline session handling with dedicated helper functions. |
| agentops/sdk/decorators/init.py | Added deprecation warning for @session decorator. |
| agentops/sdk/core.py | Integrated format_trace_id() for improved trace id formatting. |
| agentops/sdk/converters.py | Added format_trace_id() utility function with robust error handling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances SDK stability and developer experience by improving core error handling, consolidating utility logic, and cleaning up deprecated components.
- Strengthened
_flush_span_processorswith detailed exception handling and standardized trace ID formatting viaformat_trace_id(). - Removed deprecated
LiveSpanProcessorand centralized trace ID formatting in a new converter. - Refactored session tracing in decorators to helper functions and updated dependencies for Python <3.11.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_init_api_consistency.py | Added comprehensive tests for agentops.init() return consistency. |
| tests/unit/sdk/test_core_error_handling.py | New tests covering all error scenarios in _flush_span_processors. |
| pyproject.toml | Added typing_extensions dependency and defined dev-llm group. |
| agentops/sdk/types.py | Updated TracingConfig fields to use Required[int]. |
| agentops/sdk/processors.py | Removed deprecated LiveSpanProcessor class. |
| agentops/sdk/decorators/factory.py | Extracted sync/async session-trace logic into helpers. |
| agentops/sdk/decorators/init.py | Changed deprecation log level for @session and cleaned imports. |
| agentops/sdk/core.py | Enhanced force_flush error handling and integrated format_trace_id. |
| agentops/sdk/converters.py | Introduced format_trace_id utility with error handling. |
Comments suppressed due to low confidence (2)
agentops/sdk/decorators/init.py:22
- [nitpick] The
sessiondecorator no longer usesfunctools.wraps(trace), causing wrapped functions to lose metadata (name, docstring). Consider reintroducing@functools.wraps(trace)to preserve transparent decorator behavior.
def session(*args, **kwargs):
agentops/sdk/core.py:337
- The
returnunder thehasattrcheck is misaligned by one indentation level, which can cause a syntax error. Align it under theif not hasattr(self._provider, 'force_flush'):block.
return
f30cd59 to
c184be2
Compare
|
Closing this because it's scraping the bottom of the barrel. Too many conflicts to invest time into and a better quality PR is needed. |
📥 Pull Request
📘 Description
Enhanced AgentOps SDK reliability, consistency, and developer experience through comprehensive improvements across core functionality, error handling, and dependency management.
🔧 Core Infrastructure Improvements
TracingCore._flush_span_processors()with comprehensive exception handling forAttributeError,RuntimeError, and unexpected errors, enhanced logging levels, and graceful degradation to ensure span processor failures don't break application flowformat_trace_id()utility function to eliminate duplicate trace ID formatting logic across the codebase, improving maintainability and consistencyLiveSpanProcessorclass to simplify the processor architecture and reduce maintenance overhead🎯 API Consistency & Backward Compatibility
@tracedecorator with better error handling, proper cleanup in finally blocks, and improved async/sync function support@sessiondecorator while maintaining backward compatibilitytyping_extensions>=4.0.0for Python <3.11 compatibility, ensuringRequiredandNotRequiredtypes work across all supported Python versions🛡️ Reliability & Error Resilience
🧪 Testing
Comprehensive test coverage added to validate all improvements:
test_core_error_handling.py)test_init_api_consistency.py)🔄 Migration Impact
dev-llmgroup for development environments@sessionto@tracedecorator with helpful warningsThese changes address critical production issues including resource management, error propagation during shutdown scenarios, and dependency version gaps while maintaining full backward compatibility and improving the overall developer experience.